Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(functions): add map_insert function #15567

Merged
merged 11 commits into from
Nov 5, 2024

Conversation

hanxuanliang
Copy link
Contributor

@hanxuanliang hanxuanliang commented May 18, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

part of #15295

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label May 18, 2024
@hanxuanliang
Copy link
Contributor Author

However, there is such a description in Snowflake:

If updateFlag is set to TRUE, then the existing input key is updated to the input value

But in Databend, do we support dynamically setting config in the query? Alternatively, we can set the config when querying with the bendsql?

@b41sh b41sh self-requested a review May 18, 2024 05:21
@b41sh
Copy link
Member

b41sh commented May 18, 2024

However, there is such a description in Snowflake:

If updateFlag is set to TRUE, then the existing input key is updated to the input value

But in Databend, do we support dynamically setting config in the query? Alternatively, we can set the config when querying with the bendsql?

We can register a new function with four arguments for map_insert, for example st_geometryfromwkb can support two and three args.

src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
@hanxuanliang hanxuanliang force-pushed the feat/15295_map_insert branch 2 times, most recently from 085cead to f4ae992 Compare May 19, 2024 07:31
@hanxuanliang
Copy link
Contributor Author

But, I have discovered a problem now:

CREATE TABLE map_insert_test(col_str Map(String, String Null) Not Null, col_int Map(String, Int Null) Null);

SELECT map_insert(col_int, 'a', 100)
FROM map_insert_test;

For the NULL map, it is currently stuck and unable to run. However, I tried to modify the domain calculation, but it had no effect. I have no idea here 😣

@b41sh
Copy link
Member

b41sh commented May 19, 2024

But, I have discovered a problem now:

CREATE TABLE map_insert_test(col_str Map(String, String Null) Not Null, col_int Map(String, Int Null) Null);

SELECT map_insert(col_int, 'a', 100)
FROM map_insert_test;

For the NULL map, it is currently stuck and unable to run. However, I tried to modify the domain calculation, but it had no effect. I have no idea here 😣

Both val_domain and insert_value_domain may be nullable, and we need to deal with various cases, such as

match (val_domain, insert_value_domain) {
    (Domain::Nullable(NullableDomain {..}), Domain::Nullable(NullableDomain {..})) => ...,
    (Domain::Nullable(NullableDomain {..}), _) => ...,
    (_, Domain::Nullable(NullableDomain {..})) => ...,
    (_, _) => ...,
}

src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
@hanxuanliang hanxuanliang force-pushed the feat/15295_map_insert branch from d3ad954 to 35c6a6e Compare May 24, 2024 15:42
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/map.rs Outdated Show resolved Hide resolved
@b41sh b41sh added this pull request to the merge queue Nov 5, 2024
Merged via the queue into databendlabs:main with commit 3797fb6 Nov 5, 2024
74 checks passed
@b41sh
Copy link
Member

b41sh commented Nov 5, 2024

Thanks for @hanxuanliang PR. And look forward to keeping an eye on databend in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants